improve ISO 8601 parsing for temporal filter #486
Conversation
itcarroll
changed the title
Co-authored-by: Joseph H Kennedy <[email protected]>
|
Believe the pre 3.10 checks are failing due to |
Yes, using from typing import Union
def foo(bar: Union[int, float]):For from typing import Optional
def fizz(buzz: Optional[thing]): # buzz can be None or thingBut that's an easy circle back once we're happy with the implementation |
|
Test error is |
There was a problem hiding this comment.
What I don't love about the simplification here is the call to parse when it may not have been necessary (if we already had a datetime).
I think that roundtrip significantly simplifies the logic in the function; your latest version has nested branches/leafs which I find makes it harder to follow the logic all the way through without spending some time in the function.
That said, we've got really good tests now, so I'm fine with whatever implementation you prefer that passes the tests.
|
With nasa/python_cmr#31 merged, I believe we can close this as a won't fix in favor of a new PR that passes user's temporal input directly to python_cmr. Sound good? |
Fixes #330.
As suggested by @jhkennedy, earthaccess will now pass a
datetimeobject (orNone) tocmr, essentially replacing the basic string parsing thatcmrdoes (usingdatetime.strptime) with flexible and timezone aware parsing bydateutil.parser.The change created the opportunity to create a new suite of tests for possible user inputs, that includes checking exceptions.
📚 Documentation preview 📚: https://earthaccess--486.org.readthedocs.build/en/486/